Formatting with black and isort #189
Conversation
I propose to edit the PR to equally amend the already present pre-commit hook file. Once locally activated, this already can assist (at local level) en route to eventually file a PR. Be welcome to pick, edit, adjust to your preference from example below: repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v5.0.0
hooks:
- id: trailing-whitespace
exclude: ^tests/resources/
- repo: https://github.com/psf/black
rev: 25.1.0
hooks:
- id: black
- repo: https://github.com/PyCQA/flake8
rev: 7.1.1
hooks:
- id: flake8
name: "flake8 (advisory only, it does not block a commit)"
args:
- --exclude=.*/site-packages/.*
- --filename=.*\.py
- --max-line-length=90
- --show-source
- --statistics
- --count
- --exit-zero
verbose: true
always_run: true
- repo: https://github.com/pre-commit/mirrors-isort
rev: v5.10.1
hooks:
- id: isort
name: sorts imports
args: ["--profile", "black", "--filter-files", "--line-width=99"]The file defines them explicitly, in addition to templates |
Ok, good idea! I can do that, although I personally don't like to add too much to the pre-commit hooks, since it often becomes a bit too annoying. |
|
@max-models I agree with you. Each hook and test is like a droplet of glue -- if there are too many, they i) can hinder each other's action. Instead of providing a tangible benefit to the project per sé, ii) they then slow down progress. |
Agreed :) Sometimes I feel like there is so much glue that I can't even make a dirty temporary commit, so I just turn off all the pre-commit hooks instead. In any case, using the hooks is optional, and the formatting is checked with Github actions. I added a |
|
@max-models Today I learned |
Agreed, thanks for doing this pr. There will be many conflicts with existing prs but this shouldn't be a blocker because conflicts can just be resolved using "ours" and then redo the formatting after the merge, I guess. |
I'm using |
Agreed! Almost all conflicts should be fixable with this method, or simply by running |
There was a problem hiding this comment.
I think this CI workflow can largely be discarded in favour of pre-commit CI. I would only have to enable the webhook in the Settings and make it a mandatory check for the branch ruleset.
(Some others mentioned pre-commit in the discussion). Generally, it's a good idea to define your linting, formating, etc. in one place and then have pre-commit hooks locally and on the CI use them.
Less maintenance work, overall.
There was a problem hiding this comment.
I hear you, it's a preference I guess. I prefer to avoid pre-commit hooks because they sometimes just gets in the way. Also, I tend to not always have them activated, and I see the same tendency among many others. As soon as they cause a moment of annoyance, people turn them off and then the formatting goes out the window. I feel like it's simpler to just let the developers choose themselves at which stage they would like to do the formatting. The pre-commit config file is there for anyone to activate if they would like, isn't that enough?
There was a problem hiding this comment.
people can do as they wish on their machines, having a pre-commit yaml config does not change that.
The yaml config is what is read from the pre-commit.ci which will then automatically run the checks on GitHub.
There was a problem hiding this comment.
Ah, ok sorry I see what you mean now, I didn't know this was possible. I can update the CI.
There was a problem hiding this comment.
yeah so basically how this is done is:
- repo configures isort, black, etc. in pyproject/setup.cgf or explicit config files for each tool
- repo adds simple pre-commit-config.yaml see
- repo/org admin enable the pre-commit.ci webhook
- Optional: repo admin updates branch protection rules to include pre-commit webhook
Now every commit that is pushed on GitHub will be passed through isort/black/etc. and it will fail if any of the linters/formatter fail. You only have a single source of truth to worry about (the pre-commit config).
That being said for large projects, even locally, I find that it helps developing in an organised clean manner (abiding by linters, formatters, making atomic commits etc.), so I will almost certainly use pre-commit and run tests, when I commit and definitely before I push.
4 participants
Since this project is all about code formatting, I thought it would be a nice idea to also format the python code, so I ran two of the most popular python code formatters (black and isort) on the Python files in the repo.
I also added a check in the Github workflow to verify that the code is formatted with both black and isort.